Repository navigation
Send back expectation changes that are not on the CI runner's key - #153
Merged
Merged
Conversation
reviewbot/expectation_keys.py compares each changed test file's Expectations blocks
before and after: only ("cuda", 8) / ("cuda", (8, 6)) may change, other keys keep
their value (demotion to (None, None) allowed), runner values may not move >5% of
the block's scale, no tolerance may loosen. A violation is validation feedback on the
normalizer's retry budget; one that survives is flagged in the PR body.
TASK_EXPECTATION_KEY_CHECK=0 turns it off.
tarekziade
added a commit
that referenced
this pull request
Oct 12, 2026
Follow-up to #153. That check judged where a value went, not the value itself, so the red-flag PRs would pass it once reshaped onto `("cuda", 8)`. It also flagged a correct edit on the 2026-10-11 nightly. - **Converted literals are drift-checked.** When a plain literal becomes an `Expectations` block, the runner value is compared against the literal it replaces. transformers#49243 (pvt_v2) moved 45% and skipped the check. - **A different count of numbers is a regression.** transformers#47698 (dab_detr) went from 5 detections to 3. - **Degenerate runner text is rejected**: a control character (transformers#49244 fuyu, `\x04 A bus…`) or a stutter (transformers#47983 starcoder2, `def hello_world(): hello_world():`). This is `expectation_guard.degenerate_text`, which also feeds the guard's degenerate-value label. - **A sign flip no longer hides an expectation-only patch.** `_NUMBER_LITERAL` now includes a unary minus. In the guard, #47698's `-2.5104` left a stray `-`, so the patch read as a code change and no expectation rule ran on it in prod. - **Asserts on the model's input shape are exempt** from the runner-key rule, e.g. `inputs.input_ids.shape == (2, 422)`. transformers#49507 (pegasus) was sent back 3 times and published with a wrong⚠️ warning. The tokenizer doesn't depend on the GPU. The exemption needs an input name plus `.shape`/`len(`/`.size(`, so an output count is still checked. The exhausted-budget behaviour is unchanged: the PR still opens with the warning. Checked against the real base/head files of transformers PRs: | case | before | after | |---|---|---| | 8 red flags (#47698 #47750 #47840 #47983 #48955 #49143 #49243 #49244) | 7 rejected, #47698 passed | 8 rejected | | same bad values reshaped onto `("cuda", 8)` (pvt_v2, starcoder2, fuyu, dab_detr) | 4 passed | 4 rejected | | #49507 pegasus | rejected | passes | | 4 eligible (#48838 #48882 #48945 #49040) | pass | pass | `make test`: 1404 passed. Ships on deploy (`TASK_EXPECTATION_KEY_CHECK` still switches it off). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
serge's expectation updates mostly don't have the shape maintainers accept. Of the serge PRs opened in Sept–Oct 2026, the 3 value updates that touched only the CI runner's
("cuda", 8)entry all merged cleanly (huggingface/transformers#48515, #49068, #49076). The one that overwrote("cuda", None)merged with a wrong value (#48580). The open queue overwrote(None, None)(#47616, #49480), rewrote XPU values with no XPU evidence (#48955), loosened tolerances (#49143) and edited plain literals in place (#47840). The prompt already asked for a device key. This PR enforces it.reviewbot/expectation_keys.pyparses each changed test file before and after the patch (ast), maps everyExpectations({...})block key → value, and reports:("cuda", 8)/("cuda", (8, 6))(A10G, compute capability 8.6). Demotion is allowed: an existing value moved unchanged to(None, None), which is what #48198 did;(None, None)value that isn't an existing one;atol/rtol, in any patch;Expectationsblock. Test-code fixes are left alone (#47697 renamed a config key).Wiring.⚠️ Expectation change not in the maintainer shape" section listing the problems.
_validate_patchruns it after the patch applies, before the brevity pass and the normalizer. A violation resets the worktree and comes back as correction feedback (rule text included), using the sameTASK_NORMALIZE_MAX_RETRIESbudget. If a violation survives the budget, the PR still opens, with a "TASK_EXPECTATION_KEY_CHECK=0turns the check off.Checked against real PRs. Every serge PR since 2026-07-20, comparing each test file at its base and head commits:
Expectationswith the runner key.The prompt side (the guidance now names the runner key, and a routing bug kept it from reaching most assertion groups) is in huggingface/transformers-ci#215.
Tests:
tests/test_expectation_keys.py(17 cases cut down from the PRs above) andExpectationShapeValidationTestsintests/test_tasks.py. Full suite: 1390 passed.🤖 Generated with Claude Code